[rhcos-4.16] mantle/kola/tests: adapt mpath verification #4352
Conversation
The test needs to be adapted so it doesn't verify that we're on multipath based on whether the device is `/dev/mapper/mpath` or not, but instead based on udevadm output. This verification works whether or not the `user_friendly_names` option is used while configuring multipath.
In order to have a more generic test, let's not use user-friendly names while configuring multipath and specify instead a World Wide ID to the stub device. We also adapt the systemd units accordingly.
|
Hi @openshift-cherrypick-robot. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adapts the multipath tests to use non-user-friendly names, which are more stable. The changes involve updating the mpathconf command, using WWN-based device paths in systemd units, and making the multipath verification logic more robust. My review includes a fix for the multipath verification logic and a suggestion to improve maintainability by refactoring hardcoded identifiers. Overall, the changes are in the right direction.
| srcdev := string(c.MustSSHf(m, "findmnt -nvr %s -o SOURCE", path)) | ||
| if !strings.HasPrefix(srcdev, "/dev/mapper/mpath") { | ||
| udevinfo := string(c.MustSSHf(m, "udevadm info %s", srcdev)) | ||
| if !strings.Contains(udevinfo, "/dev/disk/by-id/dm-uuid-mpath-") && !strings.Contains(udevinfo, "DM_MPATH") { |
There was a problem hiding this comment.
The output of udevadm info provides symlink paths relative to /dev, not absolute paths. Therefore, the check for "/dev/disk/by-id/dm-uuid-mpath-" will always fail. The leading /dev/ should be removed to correctly check for the presence of the multipath device symlink.
| if !strings.Contains(udevinfo, "/dev/disk/by-id/dm-uuid-mpath-") && !strings.Contains(udevinfo, "DM_MPATH") { | |
| if !strings.Contains(udevinfo, "disk/by-id/dm-uuid-mpath-") && !strings.Contains(udevinfo, "DM_MPATH") { |
|
/ok-to-test |
3 participants
This is an automated cherry-pick of #4113
/assign joelcapitao